The implementation seems to be working just fine: now I am able to run dart code metrics as windows executable. I did some measurements again to check if compilation to exe improves performance.

I created two batch files:

measure.bat:

@echo %time%
@call dart dart-code-metrics/bin/metrics.dart analyze dart-code-metrics/lib --sdk-path D:\SDK\dart-sdk >nul
@echo %time%

measure_exe.bat:

@echo %time%
@call metrics.exe analyze --sdk-path D:\SDK\dart-sdk dart-code-metrics/lib >nul
@echo %time%

The results I see:

measure.bat: ~8 seconds

measure.exe.bat: ~ 2 seconds

As you can see, there is an "amazing" performance improvement, but I think this is too good to be true ;) So I think we need more measurements/comparisons to find out if we get real performance benefits when compiling to exe like:

• Do we get performance improvement if we compare exe performance with aot snapshot performance when dart code metrics is activated globally from pub? I was unable to quickly test this because the current version is not published yet, and there is no precompilation when activating from path AnalysisSession getResolvedUnit2 is slower than AnalysisServer unit resolution dart-lang/sdk#46914 (comment)
• Can you provide some measurements from your side? @dkrutskikh
• Maybe it would be good to ask the Telegram community, what they think about this, do they see performance benefit.

So let's have further discussions on this if you don't mind. I will finish this PR if we will decide to move forward with this approach.

Codecov Report

Merging #430 (1f5702d) into master (6bbf52a) will increase coverage by 0.18%.
The diff coverage is 81.57%.

❗ Current head 1f5702d differs from pull request most recent head 9edcfbc. Consider uploading reports for the commit 9edcfbc to get more accurate results

@@            Coverage Diff             @@
##           master     #430      +/-   ##
==========================================
+ Coverage   84.74%   84.92%   +0.18%     
==========================================
  Files         217      218       +1     
  Lines        4439     4459      +20     
==========================================
+ Hits         3762     3787      +25     
+ Misses        677      672       -5     

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6bbf52a...9edcfbc. Read the comment docs.

I think the result is really good and it correlates with what I measured locally (~8 seconds for compilation). Thank you for your effort!

Do we get performance improvement if we compare exe performance with aot snapshot performance when dart code metrics is activated globally from pub?

Yeah, that's a good question, I think, the answer will not surprise us 😄. We published 4.2.0 today, could you please measure the snapshot version too?

Maybe it would be good to ask the Telegram community, what they think about this, do they see performance benefit.

Please show me a person who'd say that slower is better 😄😄😄. I think that it's a right direction no matter what.

The only suggestion I have is to check how dart analyze handles SDK detection and maybe we can find a solution without a separate option. Could you please research it too?

I really like the trend in your tests: 12 secs -> 8 secs -> 2 secs. It's such a magnificent improvement!!!

@incendial, I like the trend too :) But there is still something we have to do before we get this ready to be used...

Tomorrow I will measure speed of running dart code metrics as precompiled AOT snapshot and publish the result here. We can continue discussing if running as EXE will still be significantly faster, if not - I think we can just stop here.

About Dart SDK path detection: I think, this is actually not possible. Users can have multiple Dart SDK versions installed (like I do). dart analyze runs inside Dart process so I assume it is easy to detect Dart SDK path from process handle / dart.exe path.

But when we compile to Windows EXE this information is lost and there is no any association between our EXE file and Dart SDK.

So I do not see many options at the moment:

• Use CLI option for SDK path
• Bundle required Dart SDK files as CLI application assets. But even in this case, I am not sure if Dart compiler will bundle these assets into compiled EXE file.

Actually, second scenario as I understand is not quite easy to implement. So as you see I am not quite optimistic here, but I will be back tomorrow with AOT snapshot measurements :)

Today I performed three performance measurements. All measurements were done on this branch. Here are three batch files and the results:

measure_local.bat

@echo %time%
@call dart dart-code-metrics/bin/metrics.dart analyze dart-code-metrics/lib --sdk-path D:\SDK\dart-sdk >nul
@echo %time%

measure_aot.bat (this uses globally activated dart code metrics 4.2.0 and I ignored first launch while measuring because there is time to precompile aot)

@echo %time%
@call C:\Users\roman.petrov\AppData\Local\Pub\Cache\bin\metrics.bat  analyze dart-code-metrics/lib >nul
@echo %time%

measure_exe.bat

@echo %time%
@call metrics.exe analyze --sdk-path D:\SDK\dart-sdk dart-code-metrics/lib >nul
@echo %time%

The results are:

measure_local.bat: ~8 seconds

measure_aot.bat: ~4,5 seconds

measure_exe.bat: ~2,5 seconds

So exe still seems to be fastest method from my measurements. @dkrutskikh - can you please repeat these measurements on Windows to validate my results?..

If we decide to provide some fast dart code metrics workflow with exe compilation we need to figure out first how this can be done :) Current PR with CLI option might be just a starting point.

@incendial,

A couple of days ago @dkrutskikh proposed an idea: we can try to detect SDK path from system PATH environment variable. I think this might be quite a good approach. The logic can be as follows:

• for each <directory> in PATH try to find dart.exe
• if found - use parent directory of this <directory> as SDK path

The possible drawback: some users (like I do) can have multiple Dart SDK versions installed and SDK we found might be outdated. In this case, users will just get unpredictable runtime errors when running compiled exe. But I still think that this approach is a good trade-off between complexity and functionality.

But the most important question for me is: will Windows users actually use compiled exe? We know, compiling to exe gives a significant performance improvement. But it involves non standard Dart workflow:

• Standard workflow: to use/upgrade dart package it is required only to run dart pub global activate <package_name> and that's all.
• EXE workflow: users will have to clone the repository or activate the package globally first to download the package source code. After that, they should compile it using dart compile exe. The upgrade process requires repeating these steps.

So I am completely unsure if users will like the second workflow and use it. This complex workflow might provide major benefits only for large codebases. Personally, I am quite satisfied with CLI performance after #428.

@incendial , @dkrutskikh - what do you think, should we continue with this?

The possible drawback: some users (like I do) can have multiple Dart SDK versions installed and SDK we found might be outdated. In this case, users will just get unpredictable runtime errors when running compiled exe. But I still think that this approach is a good trade-off between complexity and functionality.

This exactly why we need to combine those approaches: we will provide auto-detection based on PATH, but if it'll not work for the user in a specific case (like having multiple SDK version) we should also provide a cli option in order to let the user get a working exe and not be blocked.

EXE workflow: users will have to clone the repository or activate the package globally first to download the package source code. After that, they should compile it using dart compile exe. The upgrade process requires repeating these steps.

I think we can choose an approach with package managers and distribute precompiled version like fvm does, for example. This approach will not require any action from the user, but to download the precompiled executable and call it.

But we can't move to this step until we have no doubts about the package working correctly and without SDK detection it won't work, so I think we should definitely finish this improvement and then consider a distribution through package managers.

So:

• we will use the --sdk-path option that I already implemented.
• I will update this PR and implement sdk path detection from PATH environment variable: this should work if the application process started on Windows as .exe file
• .exe distribution will be not part of this PR and should be discussed/implemented in another issue?

@incendial, is that correct?..

Yeah, that's how I imagined it. Completing the first two steps will unblock use-cases with local compilation (like you have) and let you speed up the pipeline. Then we'll move forward to improving the DX with precompiled distribution.

Will this work?

I think this would be ok and I like the result we got. I will continue my work on this PR and implement sdk path detection from PATH.

BTW, I will have a two-week vacation in a few days, so I think this PR will be completed only after that. Please let me know if it's ok or just feel free to take ownership of this branch if you like to deliver it faster.

BTW, I will have a two-week vacation in a few days, so I think this PR will be completed only after that.

Sure, no problem.

@incendial I completed Dart SDK detection functionality here. I tested EXE compilation on my machine and the compiled application seems to be working just fine without --sdk-path argument.

Can you please take a look if the detection logic fits our needs?

We had some discussion in Telegram today and I've got just one more question here: should we support non Windows platforms somehow when compiling to native executables?...

@incendial , @dkrutskikh

We had some discussion in Telegram today and I've got just one more question here: should we support non Windows platforms somehow when compiling to native executables?...

@incendial , @dkrutskikh

Today I installed Dart on Ubuntu (I have almost no experience in *nix) development. I thought that dart compile on the non-Windows platform is able to build native executables. When I ran dart compile I see

So if I understand correctly, exe is the only native executable format and it is not possible to build a native executable for *nix platforms (even running dart on these platforms)? Do you have any knowledge in this area? (as I said, I am too inexperienced in *nix)

@roman-petrov the docs says that exe can be run on different platforms https://dart.dev/tools/dart-compile#exe and I've tested it on macos and it looks fine. So, If there is nothing else you want to add/change in the current implementation, I think we can merge it.

@incendial - it's a good news. Do you have any code comments for current implementation?

I see that codecov found coverage regression, I will try to add some tests before we merge.

@incendial , sure, I will add the link to the https://dart.dev/tools/dart-compile#exe somewhere, if I understand correctly, do we need it in user docs?

@roman-petrov I think adding it into the description of the argument should be enough https://github.com/dart-code-checker/dart-code-metrics/pull/430/files#diff-34a9cc97d58062ac8d9ab552da6d66e78e8e3d41a0d1859c7561c8115d5d2366R47

@incendial - thank you, I will add the link. In a few days I will update this PR and add tests too.

I still have a question about non-Windows platforms. At the moment as you can see in the code Dart SDK detection is explicitly enabled only for Windows platform.

You said you tested this PR on macOS and it works fine too. So I am a bit stuck here, how my implementation can affect macOS... I assume that for non-Windows platforms this PR does nothing more than adding the --sdk-path CLI parameter, Dart SDK detection is disabled. Did you perform the test on macOS using this new parameter, can you please clarify?...

@roman-petrov okay, now I got your point. Yeah, I've tested compilation without the new parameter and the idea "it should work properly on any platform" was about the compilation too.

So, yeah, let's keep mentioning Windows for now and when we'll return to distributing compiled version we'll update the parameter if needed.

@incendial - sorry for late response. I was ill and still recovering, hope to get well and finish this this or next week.

No problem, I hope you'll get better soon!

I am finally very excited to get well and get back to work:) I finished working on this, PTAL @incendial , @dkrutskikh

I addressed review comments, added tests and also moved some duplicated code into BaseCommand class. Still see some duplication between three commands we have, but this seems to be out of my pull request scope.

I will make follow up PR into website repository once this gets merged.

@roman-petrov sorry, but I'm a bit overloaded right now, I'll try to reply to all your comments and PR's in a few days.

@incendial - no problem, take your time.